Skip to content

ENH: EA._cast_pointwise_result #62105

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

jbrockmendel
Copy link
Member

@jbrockmendel jbrockmendel commented Aug 14, 2025

Still have a bunch of pyarrow tests involving duration/timestamp dtypes failing. Also need to update/remove the test files' _cast_pointwise_result methods.

xref #56430, could close that with a little effort.

I suspect a bunch of "pyarrow dtype retention" tests are solved by this, will update as I check. Nope!

@jbrockmendel jbrockmendel changed the title ENH: EA._cast_pointwise_result WIP/ENH: EA._cast_pointwise_result Aug 14, 2025
@jbrockmendel
Copy link
Member Author

The pyarrow duration stuff is caused by an upstream issue apache/arrow#40620

@jbrockmendel jbrockmendel changed the title WIP/ENH: EA._cast_pointwise_result ENH: EA._cast_pointwise_result Aug 15, 2025
@jbrockmendel
Copy link
Member Author

@rhshadrach i think you had a recent issue/pr involving retaining nullable dtypes in a .map?

Copy link
Member

@rhshadrach rhshadrach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really great improvement here. It seems to me there are two potential situations we might find ourselves in: (a) take the dtype of self into account or (b) don't take the dtype of self into account. I highlighted one specific example below where I think this might go awry. The base implementation is doing (b) whereas the subclasses are doing various degrees of (a). Understand that isn't being introduced here, but I think the long term goal is to make this more consistent?

For now, do we want to setup the framework to separate these two cases out somehow - perhaps an argument to _cast_pointwise_result?

I see now that this is just preserving the dtype when possible. I think we don't need two different cases as I first imagined.

@@ -282,33 +281,10 @@ def _create_arithmetic_method(cls, op):
DecimalArrayWithoutCoercion._add_arithmetic_ops()


def test_combine_from_sequence_raises(monkeypatch):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why remove?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

its impossible to make this test work since it defines _from_sequence to always raise. which strikes me as an unnecessary case to test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

REF: make _cast_pointwise_result an EA method
3 participants